-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SD-JWT VC Profile #115
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Direction generally looks good but imo, the matcher algorithm is missing. You cannot apply JSONPath using PE to the SD-JWT (i.e., issuer-signed JWT plus disclosures). But the wallet can apply the matcher using the JSONPath to the "Unsecured Payload of an SD-JWT VC" (see https://www.ietf.org/archive/id/draft-ietf-oauth-sd-jwt-vc-03.html#section-1.4). After this was added, I'm fine approving it.
You actually can apply a JSONPath to the SD-JWT VC but it would not give you the expected result since it contains all the _sd
claims and not the actual disclosures in one JSON object.
please also review this PR in HAIP that removes sd-jwt vc profile from HAIP openid/oid4vc-haip-sd-jwt-vc#96 |
it might be beneficial to adopt for sd-jwt vc a similar processing requirements as in 18013-7 for mdocs: ""path is a JSON array where each entry is a JSON String containing a requested data element from the requested document type as follows: |
That would work too. Or just say that the root of the JSONPath is the Unsecured SD-JWT VC Payload. I'm ok with both options as long it is defined. |
Co-authored-by: Brian Campbell <[email protected]>
…quested claims defined in PE
That definition is quite confusing and took me a while to process (part of which is caused by the use of "JSON" here). {
"org.iso.18013.5.1": {"family_name": "Doe", "given_name": "John"},
"domestic_namespace": {"foo": "bar"}
} Is that right? How would that fit SD-JWT VC? |
Co-authored-by: Christian Bormann <[email protected]>
Co-authored-by: Christian Bormann <[email protected]>
There are a few places in the Introduction, Terminology, and Overview sections that have text along the lines of this:
which should probably be updated to also mention SD-JWT VC with this PR that adds a profile for it. |
@bc-pi can that be a separate PR..? |
As per Kristina's comment opened an issue here to track this: #143 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved when my remaining two suggestions are considered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved on the basis we resolve the duplication of supported algorithms as described in #136 separately.
Co-authored-by: Kristina <[email protected]>
Co-authored-by: Paul Bastian <[email protected]>
Co-authored-by: Oliver Terbu <[email protected]>
Co-authored-by: Oliver Terbu <[email protected]>
Co-authored-by: Oliver Terbu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
PR to resolve #74